otel: replace cobraotel with native lifecycle management by Jdepp007004 · Pull Request #3108 · authzed/spicedb

Jdepp007004 · 2026-05-09T14:47:26Z

Description

Testing

docker-compose up --build, then send a few requests to the server and go to localhost:3000. You should be able to see traces
shutdown the docker containers. You should see this log: "shutting down OTel provider"

tstirrat15

See comments

tstirrat15 · 2026-05-13T19:23:29Z

 func RegisterCommonFlags(cmd *cobra.Command) {
-	otel := cobraotel.New("spicedb")
-	otel.RegisterFlags(cmd.Flags())
+	f := cmd.Flags()


I like this

Thanks, glad it reads better there

The code in RegisterOTelFlags looks awfully similar :)

I think this code can be deleted. None of the other commands (other than spicedb serve) use the otel flags.

EDIT: i've gone ahead and deleted it.

tstirrat15 · 2026-05-13T19:25:18Z

+	cmd.SetContext(context.Background())
+	require.NoError(t, cmd.Flags().Set("otel-provider", "otlpgrpc"))
+	require.NoError(t, cmd.Flags().Set("otel-endpoint", "localhost:4317"))
+	require.NoError(t, cmd.Flags().Set("otel-insecure", "true"))


I generally like the tests. Can we add a test or two that establishes 1. that you can use the otel environment variables to configure flags that aren't set and 2. which value overrides which if you declare both?

Done, added TestOTelConfig_EnvVarConfiguresUnsetFlag and TestOTelConfig_ExplicitFlagOverridesEnvVar to the integration test file. First one verifies the SDK picks up OTEL_EXPORTER_OTLP_ENDPOINT when the endpoint flag is not explicitly set, second one documents that an explicit flag value takes precedence over the env var.

I removed these tests because they weren't really asserting anything useful

miparnisari

Would it be possible to add a new struct within

spicedb/pkg/cmd/server/server.go

Line 63 in ee7c9a7

type Config struct {

called otelConfig or something like that, so that anyone doing server.NewConfigWithOptionsAndDefaults can set these programmatically?

codecov · 2026-05-15T12:40:08Z

Codecov Report

❌ Patch coverage is 82.35294% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.02%. Comparing base (49ef12e) to head (ccb9fda).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/cmd/server/otel.go	86.03%	10 Missing and 3 partials ⚠️
pkg/cmd/server/server.go	50.00%	2 Missing and 1 partial ⚠️
pkg/cmd/serve.go	33.34%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3108      +/-   ##
==========================================
+ Coverage   75.99%   76.02%   +0.04%     
==========================================
  Files         566      567       +1     
  Lines       64277    64370      +93     
==========================================
+ Hits        48840    48930      +90     
- Misses      11764    11767       +3     
  Partials     3673     3673

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Jdepp007004 · 2026-05-15T12:49:50Z

Addressing miparnisari's review — added OTelConfig as a struct in the server package with an OTel field on Config so it can be set programmatically without going through Cobra flags. InitOTelProvider now takes that struct directly along with a context and has no Cobra dependency at all.

Fixes authzed#712 and authzed#3095. - Remove dependency on github.com/jzelinskie/cobrautil/v2/cobraotel - Replicate OTel provider initialization natively in pkg/cmd/server/otel.go - Wire TracerProvider into serve.go signal handler so Shutdown and ForceFlush are called on SIGTERM/SIGINT, preventing span loss on exit - Fix vendored cobrautil Viper global singleton bug: viper.SetEnvPrefix was mutating global state instead of the local instance (v.SetEnvPrefix) - Touch pkg/cmd/util/util.go only to break import cycle between pkg/cmd/util and pkg/cmd/server; all flag registrations unchanged - Add 20 tests across unit, integration, and system build tags

- use ctxkey package for context key instead of a bare struct type - drop legacy otel-jaeger-* flags - collapse ShutdownOTelProvider to a single context shared across ForceFlush and Shutdown - move OTel initialization out of the Cobra layer into Config.Complete via an OTelConfig struct, matching the pattern used by other components - register provider shutdown with closeables so it participates in the ordered server shutdown sequence - add env var precedence tests

… code

miparnisari · 2026-05-26T01:06:03Z

Do we really need three different files for tests?

EDIT: I've simplified this

miparnisari

Approving, but i'm biased because I made changes 😄

miparnisari · 2026-05-26T01:41:39Z

-      --otel-endpoint string                                                  OpenTelemetry collector endpoint - the endpoint can also be set by using enviroment variables
-      --otel-insecure                                                         connect to the OpenTelemetry collector in plaintext
-      --otel-provider string                                                  OpenTelemetry provider for tracing ("none", "otlphttp", "otlpgrpc") (default "none")
-      --otel-sample-ratio float                                               ratio of traces that are sampled (default 0.01)
-      --otel-service-name string                                              service name for trace data (default "spicedb")
-      --otel-trace-propagator string                                          OpenTelemetry trace propagation format ("b3", "w3c", "ottrace"). Add multiple propagators separated by comma. (default "w3c")


FYI - serve still has them. All the other commands don't use these values.

tstirrat15

See comment, otherwise LGTM

tstirrat15

LGTM

Jdepp007004 requested a review from a team as a code owner May 9, 2026 14:47

github-actions Bot added area/cli Affects the command line area/dependencies Affects dependencies area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels May 9, 2026

Jdepp007004 closed this May 9, 2026

Jdepp007004 reopened this May 9, 2026

Jdepp007004 changed the title ~~removes vendor directory from tracking~~ otel: replace cobraotel with native lifecycle management May 9, 2026

github-actions Bot locked and limited conversation to collaborators May 9, 2026

authzed unlocked this conversation May 13, 2026

tstirrat15 reviewed May 13, 2026

View reviewed changes

miparnisari requested changes May 13, 2026

View reviewed changes

Jdepp007004 added 3 commits May 25, 2026 16:51

remove vendor directory from tracking

e4eca75

miparnisari force-pushed the fix/otel-lifecycle-native branch from edc506d to 7a2514c Compare May 25, 2026 23:51

chore: update changelog, docker-composes, documentation, and simplify…

7c1a4b0

… code

miparnisari reviewed May 26, 2026

View reviewed changes

chore: simplify tests for otel

e8c53b6

miparnisari previously approved these changes May 26, 2026

View reviewed changes

miparnisari reviewed May 26, 2026

View reviewed changes

This was referenced May 26, 2026

Configure Opentelemetry solely through OTEL_ flags #712

Closed

Missing defaults in configs when using SpiceDB as a library #2502

Open

tstirrat15 previously approved these changes May 26, 2026

View reviewed changes

Comment thread pkg/cmd/server/otel.go

chore: add option generator

ccb9fda

miparnisari dismissed stale reviews from tstirrat15 and themself via ccb9fda May 26, 2026 16:58

miparnisari enabled auto-merge (squash) May 26, 2026 16:59

tstirrat15 approved these changes May 26, 2026

View reviewed changes

miparnisari merged commit af55756 into authzed:main May 26, 2026
44 of 45 checks passed

github-actions Bot locked and limited conversation to collaborators May 26, 2026

Conversation

Jdepp007004 commented May 9, 2026 • edited by miparnisari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tstirrat15 May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Jdepp007004 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tstirrat15 May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Jdepp007004 May 15, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari May 26, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jdepp007004 commented May 15, 2026

Uh oh!

miparnisari May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miparnisari left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miparnisari May 26, 2026

Choose a reason for hiding this comment

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Jdepp007004 commented May 9, 2026 •

edited by miparnisari

Loading

miparnisari May 26, 2026 •

edited

Loading

codecov Bot commented May 15, 2026 •

edited

Loading

miparnisari May 26, 2026 •

edited

Loading

miparnisari left a comment •

edited

Loading